Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make docker build working with after a fresh checkout #72

Closed
wants to merge 9 commits into from
Closed

make docker build working with after a fresh checkout #72

wants to merge 9 commits into from

Conversation

zerdos
Copy link

@zerdos zerdos commented Mar 7, 2019

No description provided.

@@ -224,7 +224,7 @@ const ensureCloned = register("vscode:clone", async (runner) => {
}

runner.cwd = vscodePath;
const checkout = await runner.execute("git", ["checkout", "tags/1.31.1"]);
const checkout = await runner.execute("git", ["checkout", "tags/1.31.1"]); //TODO: this tag should come from a parameter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by a parameter?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess he means it shouldn't be hard coded.

@zerdos zerdos requested a review from nhooyr as a code owner March 7, 2019 20:26
@@ -1,4 +1,4 @@
FROM node:8.15.0
FROM node:8.9.3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change?

@@ -18,7 +18,7 @@ RUN yarn && yarn task build:server:binary
# We deploy with ubuntu so that devs have a familiar environemnt.
FROM ubuntu:18.10
WORKDIR /root/project
COPY --from=0 /src/packages/server/cli-linux /usr/local/bin/code-server
COPY --from=0 /src/packages/server/cli-linux-x64 /usr/local/bin/code-server
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

afaik, there shouldn't be a -x64 suffix?

@kylecarbs

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be - that is actually the only output

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup. This was recently changed without me knowing. It's fixed in #109.

@nhooyr
Copy link
Contributor

nhooyr commented Mar 7, 2019

I don't understand what this PR fixes. How does docker build . not work after a fresh checkout?

@nhooyr
Copy link
Contributor

nhooyr commented Mar 7, 2019

See #72

@nhooyr nhooyr closed this Mar 7, 2019
@zerdos
Copy link
Author

zerdos commented Mar 7, 2019

This PR makes the Dockerfile actually build without error :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants